feat: implement issue #680 — pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch#682
Conversation
…quired/cancelled checks → false 'not passing', blocks auto-dispatch
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Code Review
This pull request introduces the pr-auto-review helper scripts and associated bats tests to determine whether a PR's CI checks satisfy the passing-gate before dispatching the review agent. The review feedback focuses on enhancing the robustness of the jq parsing logic within ready-check.sh to prevent fatal errors when dealing with null, missing, or malformed JSON payloads, along with adding corresponding test coverage in the bats suite.
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 5dbc9b0eebe5bf08dc83f3e5107998c96f97df5a
Review mode: triage-approved (single reviewer)
Summary
Implements issue #680 (option A with option B fallback): the pr-auto-review ready-check now gates auto-dispatch on the base branch's REQUIRED status-check contexts only, so non-required/cancelled advisory contexts (e.g. superseded dev-lead / ci-relay runs) no longer block dispatch. Decision logic is extracted into a pure, side-effect-free bash/jq helper with a 16-case bats suite and a new CI workflow gate, following the house Layer-0 pattern. All CI green, all review threads resolved, triage assessment confirmed.
Linked issue analysis
Closes #680 — pr-auto-review ready-check counts non-required/cancelled checks as "not passing", blocking auto-dispatch.
All acceptance criteria are met:
- Non-required/cancelled advisory contexts no longer block:
pr_auto_review_checks_readygates on the branch ruleset's required contexts only (fetched viaGET /repos/{owner}/{repo}/rules/branches/{branch}); the exact #678 repro (cancelleddev-lead / ci-relay+dev-lead / dispatchalongside green required checks) is pinned by a dedicated bats test. - Required-check failure/pending still blocks: failing, pending, cancelled, and not-yet-reported required contexts each return not-ready (all tested).
- Decision logic unit-tested: logic lives in
.github/scripts/pr-auto-review/lib/ready-check.sh(pure functions) withtest/workflows/pr-auto-review/ready-check.bats(16 tests) run by the newpr-auto-review-tests.ymlworkflow, which triggered and passed on this PR. - The issue's recommended option A was implemented, with option B (fail/pending-only gate, cancel non-blocking) as the documented fallback when no required contexts are configured — matching the issue's guidance.
Findings
No blocking findings.
Security review of the workflow changes (all clean):
- New checkout step:
actions/checkoutpinned to9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0, verified via the GitHub API as the exact commit for tagv7.0.0(repo standard: SHAs looked up, not guessed). GH_PAT_WORKFLOWSis already a declared secret of this reusable workflow onmain(used for GH_TOKEN in existing steps) — no new credential surface.persist-credentials: falseis set, and the checkout ref isgithub.job_workflow_sha(the reusable's own pinned commit), so the sourced script always matches the pinned workflow version and cannot be swapped by a caller.- No script-injection vectors:
BASE_BRANCHcomes from trustedgh pr viewoutput and is used as a quoted API path argument;REQUIRED_JSONis jq-produced compact JSON passed via--argjsonwith[]fallbacks on API/parse failure. - Secret scan:
run_secret_scanningMCP tool not available in this environment; gitleaks CI check passed. No secrets in the diff.
Correctness notes (non-blocking):
- The name-matching heuristic (bare context vs
"workflow / job", suffix match in both directions) could in theory match a same-named job from a different workflow, but this is documented in the README and is the pragmatic trade-off given how rulesets store contexts. - Robustness feedback from the prior gemini review (null
.parameters, non-string.name/contexts, empty-arg defaults) was applied in commit5dbc9b0and is covered by tests; all 4 review threads are resolved.
CI status
All checks green at 5dbc9b0eebe5bf08dc83f3e5107998c96f97df5a: CI (Lint, ShellCheck, Secret scan/gitleaks, Agent Security Scan), CodeQL (Analyze actions), SonarCloud (Quality Gate passed: 0 new issues, 0 hotspots), AgentShield, Dependency audit, PR Auto-Review Tests (Lint and bats — new gate introduced by this PR), CodeRabbit. Skipped: dependabot-automerge, ecosystem-specific audits, dev-lead ci-relay (expected). Mergeable: MERGEABLE; merge state BLOCKED only on the pending review decision.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.



Closes #680
Implemented by dev-lead agent. Please review.